Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent new RentPaying state created by paying fees #23358

Merged

Conversation

CriesofCarrots
Copy link
Contributor

Problem

Because fees are deducted on account load, before transaction execution, the RentPaying-transition checks from #22292 do not catch accounts that are left in a newly RentPaying state due to fees.

Summary of Changes

Call a version of check_rent_state() from Accounts::load_transaction, and fail invalid rent-state transitions the same way as other fee-account failures.

Note: generally, transactions that take an account to an Uninitialized state are allowed. However, this changeset fails transactions wherein a fee-payer account has a small enough balance that the fee would make the account RentPaying and the transaction would empty the account. This is because we need to check fee-payers before we know what would happen in the transaction. This seems okay to me, but curious to hear your thoughts, or if you can see a quick workaround.

Fixes #23342

jstarry
jstarry previously approved these changes Feb 26, 2022
Copy link
Member

@jstarry jstarry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just a few nits

runtime/src/account_rent_state.rs Outdated Show resolved Hide resolved
runtime/src/account_rent_state.rs Outdated Show resolved Hide resolved
runtime/src/accounts.rs Outdated Show resolved Hide resolved
runtime/src/accounts.rs Outdated Show resolved Hide resolved
runtime/src/bank.rs Outdated Show resolved Hide resolved
@mergify mergify bot dismissed jstarry’s stale review February 26, 2022 03:58

Pull request has been modified.

@codecov
Copy link

codecov bot commented Feb 26, 2022

Codecov Report

Merging #23358 (b06bde0) into master (533eca3) will increase coverage by 0.0%.
The diff coverage is 95.5%.

@@           Coverage Diff            @@
##           master   #23358    +/-   ##
========================================
  Coverage    81.3%    81.4%            
========================================
  Files         572      572            
  Lines      155874   156057   +183     
========================================
+ Hits       126823   127031   +208     
+ Misses      29051    29026    -25     

@CriesofCarrots CriesofCarrots merged commit 36484f4 into solana-labs:master Feb 26, 2022
mergify bot pushed a commit that referenced this pull request Feb 26, 2022
* Add failing test

* Check fee-payer rent-state change on load

* Add more test cases

* Review comments

(cherry picked from commit 36484f4)

# Conflicts:
#	runtime/src/account_rent_state.rs
@CriesofCarrots CriesofCarrots deleted the rent-paying-fee-payer branch February 26, 2022 21:00
mergify bot added a commit that referenced this pull request Feb 26, 2022
…#23360)

* Prevent new RentPaying state created by paying fees (#23358)

* Add failing test

* Check fee-payer rent-state change on load

* Add more test cases

* Review comments

(cherry picked from commit 36484f4)

# Conflicts:
#	runtime/src/account_rent_state.rs

* Fix conflicts

Co-authored-by: Tyera Eulberg <[email protected]>
jeffwashington pushed a commit to jeffwashington/solana that referenced this pull request Mar 4, 2022
* Add failing test

* Check fee-payer rent-state change on load

* Add more test cases

* Review comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Possible to create new rent-paying accounts
2 participants